feat(config): add customGetElementError config option #452

aw-davidson · 2020-02-12T23:51:47Z

Closes #360

What:
Adding a showLogOnFail option to configure logging for getBy* and getAllBy* when the query fails.

Why:
Logging the entire document when working with larger components can obscure other error messages. I often find these explicit error messages are more helpful than scrolling through the output from prettyDom. If the DOM needs to be examined then debug() can be used. @benmonro also mentions that this makes working with Testcafe harder.

How:
getElementError looks at the config and determines what to output as an error:

function getElementError(message, container) {
  const errorMessages = getConfig().showLogOnFail
    ? [message, prettyDOM(container)]
    : [message]
  return new Error(errorMessages.filter(Boolean).join('\n\n'))
}

Checklist:

Documentation added to the
docs site
I've prepared a PR for types targeting
DefinitelyTyped
Tests
Ready to be merged

codesandbox-ci · 2020-02-12T23:52:44Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 31040e5:

Sandbox	Source
adoring-liskov-6v6yb	Configuration

codecov · 2020-02-12T23:55:25Z

Codecov Report

Merging #452 into master will not change coverage by %.
The diff coverage is n/a.

@@            Coverage Diff            @@
##            master      #452   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           22        22           
  Lines          385       385           
  Branches        91        91           
=========================================
  Hits           385       385

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cf57dcd...31040e5. Read the comment docs.

src/__tests__/get-by-errors.js

benmonro

I'd use the queries object so the list of queries doesn't need to be maintained. the other comment is a total nit. :)

benmonro · 2020-02-17T15:23:52Z

src/__tests__/get-by-errors.js

+  (query, showLogOnFail) => {
+    configure({showLogOnFail})
+    document.body.innerHTML = '<div>Hello</div>'
+    expect(() => screen[query]('TEST QUERY')).toThrowErrorMatchingSnapshot()


I would prefer tests that specifically look for the presence/absence of the error message (not a fan of snapshot tests personally) but I'm not gonna block for it. I'll defer to @kentcdodds or someone else on that.

Something like this?

const div = '<div>Hello</div>' document.body.innerHTML = div try { screen[query]('TEST QUERY')) } catch (err) { expect(err.message.includes(div)).toBe(showLogOnFail) }

benmonro

Looks fine to me. I'll defer to @kentcdodds though on the snapshot tests

benmonro · 2020-02-17T17:57:49Z

Yeah that would work

eps1lon

I don't really find myself using this option. In CI you want it anyway and during dev most of the time as well. I hold off voting for this one.

benmonro · 2020-03-03T16:20:43Z

@eps1lon this is specifically useful in e2e tests where logging the entire Dom is never going to be useful. Even in CI I'd say you don't want it as the Dom will usually be higher than the limit anyway. In RTL tests sure but for testcafe, Cypress etc full Dom logging Is just noise.

benmonro · 2020-03-03T16:21:30Z

@kentcdodds any objections to merging this?

kentcdodds

This looks fine to me. Just one thing, then anyone can feel free to merge.

src/__tests__/__snapshots__/get-by-errors.js.snap

kentcdodds

Super. Thank you!

benmonro · 2020-03-03T18:51:19Z

👍

kentcdodds · 2020-03-03T18:52:23Z

Hold it. Hmm.... I just had an idea.

What if we invert control and instead hand the logging responsibility to you if you want it.

kentcdodds · 2020-03-03T18:54:14Z

So something like:

config.customGetElementError((message, container) => {
  // do whatever. So to support what you want here it'd be:
  return new Error(message)
})

I think I'd be happier with that :)

aw-davidson · 2020-03-03T19:00:09Z

@kentcdodds, Ok. I'll try implementing that

kentcdodds · 2020-03-04T00:16:01Z

src/query-helpers.js

+  const errorMessages = [message, prettyDOM(container)]
+  if (customGetElementError) {
+    return customGetElementError(...errorMessages)
+  }


I think I'd prefer it if it were this way:

Suggested change

const errorMessages = [message, prettyDOM(container)]

if (customGetElementError) {

return customGetElementError(...errorMessages)

}

if (customGetElementError) {

return customGetElementError(message, container)

}

const errorMessages = [message, prettyDOM(container)]

That way the developer writing their custom error getter can pretty-ify the container if they want, but they can do something else with it if they'd rather.

I was thinking that too, but I wasnt sure.

@kentcdodds just updated with that last suggestion and rebased.

Thanks for that!

One other thing, with this now I think we can make things even better. It's hard to explain so I hope you don't mind if I make the change myself and you can ask me questions/provide feedback if you have any.

Yeah, that refactor makes sense

closes testing-library#360

kentcdodds · 2020-03-04T02:54:15Z

🎉 This PR is included in version 6.13.0 🎉

The release is available on:

npm package (@latest dist-tag)
GitHub release

Your semantic-release bot 📦🚀

kentcdodds · 2020-03-04T03:13:21Z

@all-contributors please add @aw-davidson for code and tests

allcontributors · 2020-03-04T03:13:31Z

@kentcdodds

I've put up a pull request to add @aw-davidson! 🎉

aw-davidson changed the title ~~feat(config): add showLogOnfail config option to configure~~ feat(config): add showLogOnfail config option Feb 12, 2020

aw-davidson mentioned this pull request Feb 13, 2020

feat(testing-library__dom): Add showLogOnFail option to config - version 6.12.2 DefinitelyTyped/DefinitelyTyped#42325

Closed

8 tasks

benmonro reviewed Feb 16, 2020

View reviewed changes

src/__tests__/get-by-errors.js Outdated Show resolved Hide resolved

benmonro reviewed Feb 16, 2020

View reviewed changes

src/__tests__/get-by-errors.js Outdated Show resolved Hide resolved

benmonro reviewed Feb 16, 2020

View reviewed changes

src/__tests__/get-by-errors.js Outdated Show resolved Hide resolved

benmonro requested changes Feb 16, 2020

View reviewed changes

aw-davidson force-pushed the pr/log-output-on-fail-option branch from a8362cb to 4b2228e Compare February 17, 2020 15:12

aw-davidson requested a review from benmonro February 17, 2020 15:15

benmonro reviewed Feb 17, 2020

View reviewed changes

benmonro previously approved these changes Feb 17, 2020

View reviewed changes

benmonro requested review from kentcdodds and eps1lon February 17, 2020 15:28

eps1lon reviewed Mar 3, 2020

View reviewed changes

kentcdodds reviewed Mar 3, 2020

View reviewed changes

src/__tests__/__snapshots__/get-by-errors.js.snap Outdated Show resolved Hide resolved

aw-davidson dismissed benmonro’s stale review via 9cc7ce9 March 3, 2020 18:48

aw-davidson force-pushed the pr/log-output-on-fail-option branch from 4b2228e to 9cc7ce9 Compare March 3, 2020 18:48

kentcdodds previously approved these changes Mar 3, 2020

View reviewed changes

benmonro previously approved these changes Mar 3, 2020

View reviewed changes

aw-davidson dismissed stale reviews from benmonro and kentcdodds via 997bda7 March 3, 2020 21:51

aw-davidson force-pushed the pr/log-output-on-fail-option branch from 9cc7ce9 to 997bda7 Compare March 3, 2020 21:51

aw-davidson force-pushed the pr/log-output-on-fail-option branch from 997bda7 to 1be0482 Compare March 3, 2020 22:04

kentcdodds reviewed Mar 4, 2020

View reviewed changes

feat(config): Add customGetElementError config option

8b8307a

closes testing-library#360

aw-davidson force-pushed the pr/log-output-on-fail-option branch from 1be0482 to 8b8307a Compare March 4, 2020 01:17

aw-davidson changed the title ~~feat(config): add showLogOnfail config option~~ feat(config): add customGetElementError config option Mar 4, 2020

move config around a bit

31040e5

benmonro approved these changes Mar 4, 2020

View reviewed changes

kentcdodds merged commit d6d41f9 into testing-library:master Mar 4, 2020

kentcdodds added the released label Mar 4, 2020

allcontributors bot mentioned this pull request Mar 4, 2020

docs: add aw-davidson as a contributor #456

Merged

This was referenced Mar 28, 2020

How to get getElementError function in Custom Query #508

Closed

fix: queryHelpers.getElementError is not a function #510

Merged

feat(config): add customGetElementError config option #452

feat(config): add customGetElementError config option #452

Uh oh!

Conversation

aw-davidson commented Feb 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codesandbox-ci bot commented Feb 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Feb 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

benmonro left a comment

Choose a reason for hiding this comment

Uh oh!

benmonro Feb 17, 2020

Choose a reason for hiding this comment

Uh oh!

aw-davidson Feb 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benmonro left a comment

Choose a reason for hiding this comment

Uh oh!

benmonro commented Feb 17, 2020 via email • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eps1lon left a comment

Choose a reason for hiding this comment

Uh oh!

benmonro commented Mar 3, 2020

Uh oh!

benmonro commented Mar 3, 2020

Uh oh!

kentcdodds left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kentcdodds left a comment

Choose a reason for hiding this comment

Uh oh!

benmonro commented Mar 3, 2020

Uh oh!

kentcdodds commented Mar 3, 2020

Uh oh!

kentcdodds commented Mar 3, 2020

Uh oh!

aw-davidson commented Mar 3, 2020

Uh oh!

kentcdodds Mar 4, 2020

Choose a reason for hiding this comment

Uh oh!

aw-davidson Mar 4, 2020

Choose a reason for hiding this comment

Uh oh!

aw-davidson Mar 4, 2020

Choose a reason for hiding this comment

Uh oh!

kentcdodds Mar 4, 2020

Choose a reason for hiding this comment

Uh oh!

aw-davidson Mar 4, 2020

Choose a reason for hiding this comment

Uh oh!

kentcdodds commented Mar 4, 2020

Uh oh!

kentcdodds commented Mar 4, 2020

Uh oh!

allcontributors bot commented Mar 4, 2020

Uh oh!

Uh oh!

aw-davidson commented Feb 12, 2020 •

edited

Loading

codesandbox-ci bot commented Feb 12, 2020 •

edited

Loading

codecov bot commented Feb 12, 2020 •

edited

Loading

aw-davidson Feb 17, 2020 •

edited

Loading

benmonro commented Feb 17, 2020 via email •

edited

Loading